Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough두 개의 새로운 SwiftUI 선택 컴포넌트가 추가되었습니다: 카드형 MissionCard와 칩형 SelectionChip. 두 뷰는 텍스트와 바인딩된 선택 상태를 받아 시각적 상태를 반영하며 탭으로 선택 상태를 토글합니다. Changes
Sequence Diagram(s)(생성 조건 미충족 — UI 컴포넌트 추가만 존재하므로 다이어그램 생략) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift:
- Around line 35-41: MissionCard duplicates SelectionChip UI and tap logic;
extract the shared styling into a reusable ViewModifier (e.g.,
SelectableChipStyle) and a convenience extension (e.g.,
selectableChipStyle(isSelected:)), then replace the duplicated
background/cornerRadius/overlay code in MissionCard and SelectionChip with
.selectableChipStyle(isSelected: isSelected) while keeping the .onTapGesture {
isSelected.toggle() } where selection state is handled; ensure the modifier
accepts isSelected to switch colors and corner radius consistently across both
components.
- Line 33: MissionCard currently hardcodes .frame(width: 148, height: 80) which
duplicates SelectionChip; extract a shared constant (e.g., let sharedCardSize =
CGSize(width: 148, height: 80) or a CGFloat constants CARD_WIDTH / CARD_HEIGHT)
and replace the hardcoded .frame call in MissionCard with .frame(width:
sharedCardSize.width, height: sharedCardSize.height); also update SelectionChip
to use the same constant so both components share one source of truth and remove
duplication.
- Line 10: The MissionCard view lacks an explicit access control modifier;
update the struct declaration for MissionCard to include the intended access
level (e.g., public or internal) to match the convention used for reusable
components like SelectionChip, and ensure any related nested types/properties
used externally also have appropriate access modifiers so the component’s
visibility is consistent.
- Around line 39-41: Add VoiceOver support to the MissionCard by attaching an
accessibility label and hint and exposing its selectable state: use the existing
isSelected toggle (from the onTapGesture) to set .accessibilityLabel("...") and
.accessibilityHint("...") with context-specific text, add
.accessibilityAddTraits(.isButton) and conditionally add the selected trait when
isSelected (e.g., .accessibilityAddTraits(isSelected ? .isSelected : [])), and
implement .accessibilityAction { isSelected.toggle() } so screen reader users
can both hear the state and perform the same toggle action (follow the pattern
used in SelectionChip for wording and trait choices).
In
@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift:
- Line 10: The SelectionChip view has no explicit access control (defaults to
internal); update the declaration for struct SelectionChip to the intended
visibility (e.g., prepend public if it should be reusable across modules) and
explicitly annotate any stored properties, initializers, and methods inside
SelectionChip (and related nested types) with public or internal as appropriate
so the component's API surface is clear and consistent.
- Around line 16-20: The VStack in SelectionChip.swift wrapping the single
Text(title) is unnecessary; remove the VStack and place Text(title) directly in
the view body, preserving its modifiers (.typography(.body1_m_14) and
.foregroundStyle(isSelected ? .gray800 : .gray700)), and drop alignment:
.leading since it has no effect for a single child.
- Around line 23-27: The overlay’s RoundedRectangle stroke is applied before the
view is clipped, so the border isn’t being clipped by cornerRadius; in
SelectionChip.swift move the clipping before the overlay by applying
cornerRadius(10) or clipShape(RoundedRectangle(cornerRadius: 10)) to the view
(or container) prior to calling .overlay(...) so the stroke is clipped
correctly; update the code around SelectionChip’s overlay/cornerRadius to apply
clipping first and keep the existing isSelected ? .red500 : .gray500 stroke
logic.
- Around line 28-30: The SelectionChip view lacks VoiceOver metadata; update the
component (SelectionChip) to expose its role, state and action by adding
accessibility modifiers: set a descriptive accessibilityLabel (e.g., the chip
title), an accessibilityValue that reflects isSelected ("selected"/"not
selected"), and an accessibilityHint describing the tap action; also add an
accessibilityAddTraits(.isButton) (and .isSelected when isSelected == true) and
implement an accessibilityAction that performs the same logic as the
onTapGesture (the isSelected.toggle() behavior) so VoiceOver users can
understand and invoke the control.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swiftCherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
🧰 Additional context used
🧬 Code graph analysis (2)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Extension/Font+.swift (1)
typography(60-65)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Extension/Font+.swift (1)
typography(60-65)
🔇 Additional comments (2)
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift (1)
21-21: 고정 크기 프레임의 적절성을 검토하세요.고정된 프레임 크기(148x80)가 다양한 화면 크기와 다이나믹 타입에 대응하지 못할 수 있습니다. 디자인 요구사항에 따라 고정 크기가 필요한지, 아니면 유연한 크기 조절이 가능해야 하는지 확인하세요.
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift (1)
20-20: 이미지 에셋이 에셋 카탈로그에 존재합니다."radiobtn_selected"와 "radiobtn_default" 에셋이
Assets.xcassets/cherrishIcon/디렉토리에 정상적으로 정의되어 있으므로 런타임 오류 우려는 없습니다. 현재 코드는 문제가 없습니다.Likely an incorrect or invalid review comment.
|
|
||
| import SwiftUI | ||
|
|
||
| struct MissionCard: View { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
접근 제어자를 명시적으로 선언하세요.
SelectionChip과 마찬가지로, 재사용 가능한 컴포넌트의 접근 수준을 명확히 하기 위해 public 또는 internal을 명시적으로 선언하세요.
♻️ 제안하는 수정
-struct MissionCard: View {
+public struct MissionCard: View {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct MissionCard: View { | |
| public struct MissionCard: View { |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift
at line 10, The MissionCard view lacks an explicit access control modifier;
update the struct declaration for MissionCard to include the intended access
level (e.g., public or internal) to match the convention used for reusable
components like SelectionChip, and ensure any related nested types/properties
used externally also have appropriate access modifiers so the component’s
visibility is consistent.
| } | ||
| .padding(.horizontal, 7) | ||
| .padding(.vertical, 6) | ||
| .frame(width: 148, height: 80) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
SelectionChip과 동일한 고정 크기 프레임 문제.
SelectionChip과 동일한 프레임 크기(148x80)를 사용하고 있습니다. 디자인 요구사항을 확인하고, 두 컴포넌트가 동일한 크기를 공유해야 한다면 상수로 추출하여 중복을 제거하는 것을 고려하세요.
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift
at line 33, MissionCard currently hardcodes .frame(width: 148, height: 80) which
duplicates SelectionChip; extract a shared constant (e.g., let sharedCardSize =
CGSize(width: 148, height: 80) or a CGFloat constants CARD_WIDTH / CARD_HEIGHT)
and replace the hardcoded .frame call in MissionCard with .frame(width:
sharedCardSize.width, height: sharedCardSize.height); also update SelectionChip
to use the same constant so both components share one source of truth and remove
duplication.
| .overlay( | ||
| RoundedRectangle(cornerRadius: 10) | ||
| .stroke(isSelected ? .red500 : .gray500, lineWidth: 1) | ||
| ) | ||
| .onTapGesture { | ||
| isSelected.toggle() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
SelectionChip과 코드 중복이 있습니다.
MissionCard와 SelectionChip의 배경, 테두리, 탭 제스처 로직이 거의 동일합니다. 공통 로직을 추출하여 재사용 가능한 modifier나 base component로 만드는 것을 고려하세요.
♻️ 제안하는 리팩토링 예시
공통 스타일을 ViewModifier로 추출:
struct SelectableChipStyle: ViewModifier {
let isSelected: Bool
func body(content: Content) -> some View {
content
.frame(width: 148, height: 80)
.background(isSelected ? .red200 : .gray0)
.cornerRadius(10)
.overlay(
RoundedRectangle(cornerRadius: 10)
.stroke(isSelected ? .red500 : .gray500, lineWidth: 1)
)
}
}
extension View {
func selectableChipStyle(isSelected: Bool) -> some View {
modifier(SelectableChipStyle(isSelected: isSelected))
}
}그런 다음 각 컴포넌트에서 사용:
// MissionCard에서
var body: some View {
VStack(alignment: .leading) {
// ... content
}
.selectableChipStyle(isSelected: isSelected)
.onTapGesture {
isSelected.toggle()
}
}🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift
around lines 35 - 41, MissionCard duplicates SelectionChip UI and tap logic;
extract the shared styling into a reusable ViewModifier (e.g.,
SelectableChipStyle) and a convenience extension (e.g.,
selectableChipStyle(isSelected:)), then replace the duplicated
background/cornerRadius/overlay code in MissionCard and SelectionChip with
.selectableChipStyle(isSelected: isSelected) while keeping the .onTapGesture {
isSelected.toggle() } where selection state is handled; ensure the modifier
accepts isSelected to switch colors and corner radius consistently across both
components.
| .onTapGesture { | ||
| isSelected.toggle() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
접근성 지원을 추가하세요.
SelectionChip과 마찬가지로, VoiceOver 사용자를 위한 접근성 레이블과 힌트가 필요합니다. 라디오 버튼 동작을 수행하는 경우 .accessibilityAddTraits(.isButton)도 고려하세요.
♻️ 제안하는 수정
.onTapGesture {
isSelected.toggle()
}
+ .accessibilityLabel(missionText)
+ .accessibilityAddTraits(isSelected ? [.isButton, .isSelected] : .isButton)
+ .accessibilityHint("미션 카드를 선택하려면 탭하세요")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .onTapGesture { | |
| isSelected.toggle() | |
| } | |
| .onTapGesture { | |
| isSelected.toggle() | |
| } | |
| .accessibilityLabel(missionText) | |
| .accessibilityAddTraits(isSelected ? [.isButton, .isSelected] : .isButton) | |
| .accessibilityHint("미션 카드를 선택하려면 탭하세요") |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/MissionCard.swift
around lines 39 - 41, Add VoiceOver support to the MissionCard by attaching an
accessibility label and hint and exposing its selectable state: use the existing
isSelected toggle (from the onTapGesture) to set .accessibilityLabel("...") and
.accessibilityHint("...") with context-specific text, add
.accessibilityAddTraits(.isButton) and conditionally add the selected trait when
isSelected (e.g., .accessibilityAddTraits(isSelected ? .isSelected : [])), and
implement .accessibilityAction { isSelected.toggle() } so screen reader users
can both hear the state and perform the same toggle action (follow the pattern
used in SelectionChip for wording and trait choices).
|
|
||
| import SwiftUI | ||
|
|
||
| struct SelectionChip: View { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
접근 제어자를 명시적으로 선언하세요.
컴포넌트의 접근 제어자가 명시되지 않아 기본적으로 internal이 적용됩니다. Global/Components 디렉토리에 있는 재사용 가능한 컴포넌트이므로, 의도된 접근 수준을 명확히 하기 위해 public 또는 internal을 명시적으로 선언하는 것이 좋습니다.
♻️ 제안하는 수정
-struct SelectionChip: View {
+public struct SelectionChip: View {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct SelectionChip: View { | |
| public struct SelectionChip: View { |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
at line 10, The SelectionChip view has no explicit access control (defaults to
internal); update the declaration for struct SelectionChip to the intended
visibility (e.g., prepend public if it should be reusable across modules) and
explicitly annotate any stored properties, initializers, and methods inside
SelectionChip (and related nested types) with public or internal as appropriate
so the component's API surface is clear and consistent.
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
Outdated
Show resolved
Hide resolved
| .overlay( | ||
| RoundedRectangle(cornerRadius: 10) | ||
| .stroke(isSelected ? .red500 : .gray500, lineWidth: 1) | ||
| ) | ||
| .cornerRadius(10) |
There was a problem hiding this comment.
cornerRadius 적용 순서를 수정하세요.
overlay 이후에 cornerRadius를 적용하면 테두리(stroke)가 클리핑되지 않습니다. clipShape 또는 cornerRadius를 overlay 전에 적용해야 합니다.
🐛 제안하는 수정
.frame(width: 148, height: 80)
.background(isSelected ? .red200 : .gray0)
+ .cornerRadius(10)
.overlay(
RoundedRectangle(cornerRadius: 10)
.stroke(isSelected ? .red500 : .gray500, lineWidth: 1)
)
- .cornerRadius(10)
.onTapGesture {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .overlay( | |
| RoundedRectangle(cornerRadius: 10) | |
| .stroke(isSelected ? .red500 : .gray500, lineWidth: 1) | |
| ) | |
| .cornerRadius(10) | |
| .frame(width: 148, height: 80) | |
| .background(isSelected ? .red200 : .gray0) | |
| .cornerRadius(10) | |
| .overlay( | |
| RoundedRectangle(cornerRadius: 10) | |
| .stroke(isSelected ? .red500 : .gray500, lineWidth: 1) | |
| ) | |
| .onTapGesture { |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
around lines 23 - 27, The overlay’s RoundedRectangle stroke is applied before
the view is clipped, so the border isn’t being clipped by cornerRadius; in
SelectionChip.swift move the clipping before the overlay by applying
cornerRadius(10) or clipShape(RoundedRectangle(cornerRadius: 10)) to the view
(or container) prior to calling .overlay(...) so the stroke is clipped
correctly; update the code around SelectionChip’s overlay/cornerRadius to apply
clipping first and keep the existing isSelected ? .red500 : .gray500 stroke
logic.
| .onTapGesture { | ||
| isSelected.toggle() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
접근성 지원을 추가하세요.
컴포넌트에 접근성 레이블과 힌트가 없어 VoiceOver 사용자가 컴포넌트의 상태와 목적을 이해하기 어렵습니다.
♻️ 제안하는 수정
.onTapGesture {
isSelected.toggle()
}
+ .accessibilityLabel(title)
+ .accessibilityAddTraits(isSelected ? .isSelected : [])
+ .accessibilityHint("탭하여 선택 상태를 전환합니다")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .onTapGesture { | |
| isSelected.toggle() | |
| } | |
| .onTapGesture { | |
| isSelected.toggle() | |
| } | |
| .accessibilityLabel(title) | |
| .accessibilityAddTraits(isSelected ? .isSelected : []) | |
| .accessibilityHint("탭하여 선택 상태를 전환합니다") |
🤖 Prompt for AI Agents
In @Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
around lines 28 - 30, The SelectionChip view lacks VoiceOver metadata; update
the component (SelectionChip) to expose its role, state and action by adding
accessibility modifiers: set a descriptive accessibilityLabel (e.g., the chip
title), an accessibilityValue that reflects isSelected ("selected"/"not
selected"), and an accessibilityHint describing the tap action; also add an
accessibilityAddTraits(.isButton) (and .isSelected when isSelected == true) and
implement an accessibilityAction that performs the same logic as the
onTapGesture (the isSelected.toggle() behavior) so VoiceOver users can
understand and invoke the control.
| .overlay( | ||
| RoundedRectangle(cornerRadius: 10) | ||
| .stroke(isSelected ? .red500 : .gray500, lineWidth: 1) | ||
| ) | ||
| .cornerRadius(10) |
Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/SelectionChip.swift
Outdated
Show resolved
Hide resolved
feat: #27 chipComponent 구현
🔗 연결된 이슈
📄 작업 내용
💻 주요 코드 설명
미션카드, 선택 둘다 코드형식은 같아서 SelectionChip 코드로 사용방법 적어놓겠습니다.
코드 설명할 파일 이름 (ex: HomeView)👀 기타 더 이야기해볼 점
입주 추카합니다~~ 달려보자구요
짤은 영주온니가 선물해줘떠요 기엽조🥰